Store dimension order using new "axes" spec #121
Conversation
|
@sbesson reminded me that this needs review. Tests were added in 990b688 and conflicts are now resolved. Looks like I can't invite anyone outside the glencoesoftware org as a reviewer, but @sbesson/@will-moore in particular may wish to have a look in case what is here is inconsistent with the proposed spec change or other implementations. |
There was a problem hiding this comment.
One immediate question about scenarios where dimensionOrder would be null since axes is a required property in the multiscales dictionary.
Other the other new requirement in the multiscales metadata of the 0.4 specification is the coordinateTransformations which needs to be define at each dataset level and should minimally capture the scale information for each resolution level. I assume this will also involve reading the physical pixel size from the metadata and populating the axes unit. Depending on what is stored in the metadata, I can certainly imagine a number of scenarios, so it might be useful to define how far this implementation will need to go. Possibly we should focus on getting the axes element consolidated first and open a follow-up PR to handle the transformations?
| @@ -1514,6 +1515,27 @@ private void setSeriesLevelMetadata(int series, int resolutions) | |||
| datasets.add(Collections.singletonMap("path", lastPath)); | |||
| } | |||
| multiscale.put("datasets", datasets); | |||
|
|
|||
| if (dimensionOrder != null) { | |||
There was a problem hiding this comment.
Under which case would dimensionOrder be null? In this scenario, the multi-scale would contain no axes and be invalid or can we infer a default value?
There was a problem hiding this comment.
I think I originally assumed here that dimensionOrder can't really be null, and this is just being extra safe. Going back and looking at the history of the --dimension-order option though, dimensionOrder can be null if --dimension-order original is used. In that case, you're correct that the axes metadata is missing, so I need to fix that here.
There was a problem hiding this comment.
7595d77 should fix this so that axes is populated for any value of dimensionOrder.
|
👍 to getting this working and approved in its current state, and adding transformations in a separate PR. |
dimensionOrder is only null if "--dimension-order original" was specified.
There was a problem hiding this comment.
After realizing that the default dimension order is not the native order, I think I managed to test all possible scenarios with the following example
(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ echo "dimOrder=XYTCZ" > XYTCZ.fake.ini
(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ ./bioformats2raw-0.4.1-SNAPSHOT/bin/bioformats2raw XYTCZ.fake.ini XYZCT.zarr
OpenJDK 64-Bit Server VM warning: You have loaded library /tmp/opencv_openpnp12310768463518473249/nu/pattern/opencv/linux/x86_64/libopencv_java342.so which might have disabled stack guard. The VM will try to fix the stack guard now.
It's highly recommended that you fix the library with 'execstack -c <libfile>', or link it with '-z noexecstack'.
(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ ./bioformats2raw-0.4.1-SNAPSHOT/bin/bioformats2raw XYTCZ.fake.ini XYTCZ.zarr --dimension-order original
OpenJDK 64-Bit Server VM warning: You have loaded library /tmp/opencv_openpnp9223765734345936901/nu/pattern/opencv/linux/x86_64/libopencv_java342.so which might have disabled stack guard. The VM will try to fix the stack guard now.
It's highly recommended that you fix the library with 'execstack -c <libfile>', or link it with '-z noexecstack'.
(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ ./bioformats2raw-0.4.1-SNAPSHOT/bin/bioformats2raw XYTCZ.fake.ini XYCTZ.zarr --dimension-order XYCTZ
OpenJDK 64-Bit Server VM warning: You have loaded library /tmp/opencv_openpnp2278119605587775760/nu/pattern/opencv/linux/x86_64/libopencv_java342.so which might have disabled stack guard. The VM will try to fix the stack guard now.
It's highly recommended that you fix the library with 'execstack -c <libfile>', or link it with '-z noexecstack'.
The content of the multiscales metadata is as expected:
(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ cat XYZCT.zarr/0/.zattrs
{
"multiscales" : [
{
"metadata" : {
"method" : "loci.common.image.SimpleImageScaler",
"version" : "Bio-Formats 6.8.0"
},
"axes" : [
{
"name" : "t",
"type" : "time"
},
{
"name" : "c",
"type" : "channel"
},
{
"name" : "z",
"type" : "space"
},
{
"name" : "y",
"type" : "space"
},
{
"name" : "x",
"type" : "space"
}
],
"datasets" : [
{
"path" : "0"
},
{
"path" : "1"
}
],
"version" : "0.2"
}
]
}(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ cat XYCTZ.zarr/0/.zattrs
{
"multiscales" : [
{
"metadata" : {
"method" : "loci.common.image.SimpleImageScaler",
"version" : "Bio-Formats 6.8.0"
},
"axes" : [
{
"name" : "z",
"type" : "space"
},
{
"name" : "t",
"type" : "time"
},
{
"name" : "c",
"type" : "channel"
},
{
"name" : "y",
"type" : "space"
},
{
"name" : "x",
"type" : "space"
}
],
"datasets" : [
{
"path" : "0"
},
{
"path" : "1"
}
],
"version" : "0.2"
}
]
}(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ cat XYTCZ.zarr/0/.zattrs
{
"multiscales" : [
{
"metadata" : {
"method" : "loci.common.image.SimpleImageScaler",
"version" : "Bio-Formats 6.8.0"
},
"axes" : [
{
"name" : "z",
"type" : "space"
},
{
"name" : "c",
"type" : "channel"
},
{
"name" : "t",
"type" : "time"
},
{
"name" : "y",
"type" : "space"
},
{
"name" : "x",
"type" : "space"
}
],
"datasets" : [
{
"path" : "0"
},
{
"path" : "1"
}
],
"version" : "0.2"
}
]
}
So is the content of the resolution .zattrs
(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ cat XYZCT.zarr/0/0/.zattrs
{
"_ARRAY_DIMENSIONS" : [
"t",
"c",
"z",
"y",
"x"
]
}(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ cat XYCTZ.zarr/0/0/.zattrs
{
"_ARRAY_DIMENSIONS" : [
"z",
"t",
"c",
"y",
"x"
]
}(zarr_dev) [sbesson@pilot-zarr2-dev bioformats2raw]$ cat XYTCZ.zarr/0/0/.zattrs
{
"_ARRAY_DIMENSIONS" : [
"z",
"c",
"t",
"y",
"x"
]
}
The test changes are covering two of the three scenarios (no --dimension-order i.e. XYZCT and explicit --dimension-order). For completeness, can a third test be added to cover the --dimension-order original use case with a non XYZCT dimension order to confirm the original order is preserved? Once done, this is good to merge from my standpoint.
| } | ||
| } | ||
|
|
||
| String[] axes = |
There was a problem hiding this comment.
As mentioned yesterday and as per ome/ngff#96, this block is no longer a requirement of the specification (and is effectively broken in the default layout) so we can safely remove it.
See ome/ngff#57. Need to add a unit test before this can be merged.